Replication streaming compression (per-replica) - review vs #3531 base - #21
Open
roshkhatri wants to merge 1 commit into
Open
Replication streaming compression (per-replica) - review vs #3531 base#21roshkhatri wants to merge 1 commit into
roshkhatri wants to merge 1 commit into
Conversation
roshkhatri
force-pushed
the
replication-streaming-compression-v2
branch
4 times, most recently
from
July 29, 2026 23:07
2f851c3 to
3688b40
Compare
roshkhatri
commented
Jul 30, 2026
Comment on lines
+201
to
+216
| # Re-runs the replication suite with repl-compression=lz4-stream applied | ||
| # globally via --config. Test files opt in by carrying the top-level "repl-compression" | ||
| # tag (see tests/support/server.tcl), so this job selects them with | ||
| # --tags repl-compression instead of a hardcoded filename list that would | ||
| # silently drift if a file is renamed. Exercises the streaming-compression | ||
| # transport across the replication surface (full sync, dual-channel, buffer | ||
| # management, AOF-sync, etc.) to catch regressions that only surface under | ||
| # compressed replication. | ||
| # Note: a few tests are intentionally NOT tagged for this job: | ||
| # - integration/replication-psync: its start_server lives inside a reusable | ||
| # proc, incompatible with a top-level-only tag. | ||
| # - integration/replication-buffer.tcl and the buffer-memory tests in | ||
| # integration/dual-channel-replication.tcl: they assert exact replication | ||
| # buffer/backlog memory and byte volumes, which compression legitimately | ||
| # changes (per-replica codec buffers, fewer wire bytes). They run in the | ||
| # regular (uncompressed) job. |
| return 0; | ||
| } | ||
|
|
||
| /* Sink path: compress directly into the caller's sds tail, no scratch/emit. */ |
Owner
Author
There was a problem hiding this comment.
remove , no scratch/emit.
|
|
||
| /* Set by the repl-compression apply callback when the setting changes. Switching | ||
| * a live link between plaintext and compressed requires a reconnect, which is | ||
| * irreversible, so the reconcile is deferred until after the whole CONFIG SET |
| /* Switching existing replicas between plaintext and compressed requires a | ||
| * reconnect, which is irreversible. Only record intent here; configSetCommand | ||
| * runs the reconcile after the command commits, so a rolled-back CONFIG SET | ||
| * disconnects nothing. */ |
| /* Primary client with a live replication decoder (probing or compressed) | ||
| * must be decoded on the main thread; the IO-thread read path does not | ||
| * invoke replDecompressQueryBuf. Once the probe resolves to plaintext the | ||
| * decoder is destroyed and reads return here. */ |
| /* Check for unsent compressed data in the compressed output buffer. | ||
| * Skip while CLIENT_PENDING_IO: the IO thread owns out_buf and may | ||
| * realloc it, so the main thread must not read it here | ||
| * (postWriteToReplica re-checks once the write job completes). */ |
| * malicious or buggy primary; disconnect rather than allocate unbounded. */ | ||
| #define REPL_STREAM_DECODER_OUTPUT_MAX (256 * 1024 * 1024) | ||
|
|
||
| int replDecompressQueryBuf(client *c, size_t new_data_start) { |
Owner
Author
There was a problem hiding this comment.
Does this belong to some replication file?
Comment on lines
+89
to
+99
|
|
||
| /* Reconcile each replica's transport with the current repl-compression setting | ||
| * after a runtime change. A live link cannot switch between plaintext and a | ||
| * compressed VCS/LZ4 frame mid-stream, so a mismatched replica is dropped and | ||
| * reconnects to renegotiate: | ||
| * - compression disabled: drop replicas still on a compressed stream; | ||
| * - compression enabled: drop online replicas that advertised the capability | ||
| * but are still plaintext, so they come back compressed. | ||
| * Disconnect is async because we run inside the main event loop; the iteration | ||
| * is safe since freeClientAsync does not remove from server.replicas | ||
| * synchronously. Replicas still syncing are left alone: they pick up the current |
Comment on lines
+131
to
+134
| /* If we are ourselves a replica, the compression capability was negotiated | ||
| * with our primary at handshake time. A config change must renegotiate, so | ||
| * drop the upstream link; the replication cron reconnects and re-advertises | ||
| * capa with the new setting. */ |
roshkhatri
force-pushed
the
replication-streaming-compression-v2
branch
2 times, most recently
from
July 30, 2026 07:52
beaa970 to
5d2009b
Compare
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
roshkhatri
force-pushed
the
replication-streaming-compression-v2
branch
from
July 30, 2026 08:24
5d2009b to
5442bed
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review-only PR: isolates the per-replica replication-stream compression work, squashed to one commit on sarthak's latest valkey-io#3531 base (3975e1a, post compression-layer refactor).
Supersedes #19 (old base lineage). Does not update the upstream PR.